test: simplify test-async-hooks-http-parser-destroy #26963
Conversation
Trott
added
http
|
@Trott Sadly, an error occurred when I tried to trigger a build. :( |
It would be nice if the assert output for sets could be improved. For example, in the quoted example 159, 162, 165, 168 are listed in both |
|
@richardlau there is an open issue about this and it is on my TODO list. It is sadly just not an easy task. I need meta information about the structure of the inspected object. Otherwise it's not possible to reliable compare the correct objects. As soon as I have that, I would also like to implement a proper diffing algorithm and I already checked a couple of papers and some code about that as well. |
|
The good news is this version of the test passes on Pi now. The bad news is it fails when run inside a worker. Investigating. |
Trott
force-pushed
the
refactor-http-parser-destroy
branch
from
68da51f to
b340477
Compare
|
At this point, I have to believe this is uncovering a real bug involving |
Trott
force-pushed
the
refactor-http-parser-destroy
branch
from
b340477 to
69cdc43
Compare
|
Rebased, force-pushed, no longer failing in a worker, but still seems to be flaky due to #26961 or related. (I added code to work around asyncIds being reused, but it appears that they also might not appear at all in the destroy handler, which causes the test to timeout.) |
Simplify test-async-hooks-http-parser-destroy and improve it's reporting
when failing. (Also makes it easier to run on older versions of Node.js
because it doesn't rely on internal test modules that won't work there.)
Before, failures looked like this (edited slightly to conform to our
commit message 72-char line length restriction):
The expression evaluated to a falsy value:
assert.ok(destroyedIds.indexOf(createdAsyncId) >= 0)
Now, you get a slightly better idea of what's up. (Is it missing one ID?
More than one? All of them?):
Input A expected to strictly deep-equal input B:
+ expected - actual ... Lines skipped
Set {
1009,
...
1025,
- 158,
- 159,
- 161,
- 162,
- 164,
- 165,
- 167,
- 168,
- 170,
...
+ 159,
+ 162,
+ 165,
+ 168,
+ 171,
+ 174,
+ 177,
+ 180,
+ 183,
This test still fails as expected on 10.14.1 and passees on 10.14.2
(where the regression it tests for was fixed). (You will need to comment
on the `require('../common');` line first but that's now the only change
you need to make this run in older versions.)
Refs: nodejs#26610
Trott
force-pushed
the
refactor-http-parser-destroy
branch
from
69cdc43 to
079d054
Compare
Simplify test-async-hooks-http-parser-destroy and improve it's reporting
when failing. (Also makes it easier to run on older versions of Node.js
because it doesn't rely on internal test modules that won't work there.)
Before, failures looked like this (edited slightly to conform to our
commit message 72-char line length restriction):
Now, you get a slightly better idea of what's up. (Is it missing one ID?
More than one? All of them?):
This test still fails as expected on 10.14.1 and passees on 10.14.2
(where the regression it tests for was fixed). (You will need to comment
on the
require('../common');line first but that's now the only changeyou need to make this run in older versions.)
Refs: #26610
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes